Skip to content

Comments

Feat/fresh sesh#46

Open
BhattPrateek wants to merge 15 commits intomasterfrom
feat/fresh-sesh
Open

Feat/fresh sesh#46
BhattPrateek wants to merge 15 commits intomasterfrom
feat/fresh-sesh

Conversation

@BhattPrateek
Copy link

Duuuuuuuude. Am Happy.

@epan epan temporarily deployed to u4u-staging-pr-46 May 20, 2017 22:01 Inactive
return (
<div>
<Header />
<Header handleFav={props.route.handleFav} />
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This prop can be removed

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

}

addToFav(e) {
console.log(this.props.college);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove the console log

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.


addToFav(e) {
console.log(this.props.college);
this.props.handleFav(this.props.college);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break when this entry is being rendered for the favorites because it won't be able to properly reference handleFav.

});
}

handleFavColleges(college) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try and do 2 big things with this:

  1. Let's write it so it toggles fav colleges on multiple clicks rather than adding the college over and over.
  2. Let's see if we can make it generic and pass it to FavoritesPage.jsx OR let's write a very similar function in there that adjusted to the state in FavoritesPage.jsx and passed to ResultsListEntry.jsx . This would permit us to remove favorites from the favorites page.

this.handleFav = this.handleFav.bind(this);
}

handleFav(){
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this doesn't do anything, let's remove it.

import CommentsPage from './components/CommentsPage.jsx';

class App extends React.Component {
constructor(props){
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this if it isn't doing anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants